Tests Passed

Stats from current PR

This appears to break cache function from React.
https://stackblitz.com/edit/stackblitz-starters-a4zmeu?file=app%2Fpage.tsx

"use server"
import { cache } from "react"
export const test = cache(async () => {
 return "test"
});

The following will now crash.

import { test } from "./actions"
const CachedComp = async () => {
  const val = await test()
  return <>{val}</>
}

export default function Page() {
  return <>
    <CachedComp />
    <CachedComp />
  </>
}

Everything works on latest canary 14.1.2-canary.5

After updating to 14.1.2 we got surprised by an issue - we have an async server action function in a file marked with use server. That function was calling another function in the same file that was not async, but was exported (turns out for no reason, not using it anywhere else). I didn't get any warnings from nextjs on this, but it absolutely broke the original function since the other fn was now returning a promise instead of it's original value. Removing export from the function definition fixed that - but this was certainly unexpected.

The new fix seems to be automatically converting all exported functions in a use server file to async with no warning. Is this like...a bug? As in, should I log it somewhere? It certainly feels like a breaking change (since it did break our production build), but I'm not sure what to do with that information. If "do nothing" is the answer that's fine, I just wanted to bring it up.

After updating to 14.1.2 we got surprised by an issue - we have an async server action function in a file marked with use server. That function was calling another function in the same file that was not async, but was exported (turns out for no reason, not using it anywhere else). I didn't get any warnings from nextjs on this, but it absolutely broke the original function since the other fn was now returning a promise instead of it's original value. Removing export from the function definition fixed that - but this was certainly unexpected.

The new fix seems to be automatically converting all exported functions in a use server file to async with no warning. Is this like...a bug? As in, should I log it somewhere? It certainly feels like a breaking change (since it did break our production build), but I'm not sure what to do with that information. If "do nothing" is the answer that's fine, I just wanted to bring it up.

Same here. I noticed the issue by an unexpected returned value, otherwise there was no errors or warnings. I was able to debug the issue by logging the result returned by server action and realize that it actually returns a promise even though the function is synchronous. Apparently all functions placed in a "use server" file are automatically turned into async functions, even though definitions are sync. In my opinion it was the missing piece for use server files and definitely a great improvement.

But we need an Eslint check/error for sync functions placed in use server files.

Hey everyone, thanks for all your feedback!

Tl;DR

"use server" requires the functions to be async. If they are not, they will be converted to async functions.

So, instead of silently converting your sync functions to be async (breaks type checking), here we're directly showing errors and ask you to make them async in the first place.

Details

it breaks React.cache

Please give #62860 (comment) a read - React.cache will not do anything when it's called inside a Server Action.

but it absolutely broke the original function since the other fn was now returning a promise instead of it's original value

As per this PR description, async functions and sync functions that return promises are fundamentally different things:

const f = async () => { throw 1; return 1 }
const g = () => { throw 1; return Promise.resolve(1) }

The new fix seems to be automatically converting all exported functions in a use server file to async with no warning. Is this like...a bug?

Again, as mentioned in the PR description, "Because the underlying network calls are always asynchronous, 'use server' can only be used on async functions." (https://react.dev/reference/react/use-server) that's a requirement. Previously that's not ensured which was a bug and this PR fixes it.

But we need an Eslint check/error for sync functions placed in use server files.

This would be great, but it's difficult to ensure the type statically. Also in TS async () => {} and () => Promise are considered the same type but they're actually different things.

Agree. It also breaks the use of Zod for validating the input of Server Actions (this worked before)

Agreed that this is annoying. I'll see if there are any other work arounds.